-
Notifications
You must be signed in to change notification settings - Fork 21.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support torchbind as attribute in torch.fx symbolic tracing #48732
Conversation
This pull request was exported from Phabricator. Differential Revision: D25116185 |
031f427
to
037c4bf
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
💊 CI failures summary and remediationsAs of commit 7197f35 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 44 times. |
This pull request was exported from Phabricator. Differential Revision: D25116185 |
037c4bf
to
cd4c56e
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
cd4c56e
to
c4ffd52
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
c4ffd52
to
5a348ed
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
5a348ed
to
1cc045f
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
1cc045f
to
fd3631d
Compare
This pull request was exported from Phabricator. Differential Revision: D25116185 |
fd3631d
to
5f551bd
Compare
Codecov Report
@@ Coverage Diff @@
## master #48732 +/- ##
==========================================
- Coverage 80.78% 80.78% -0.01%
==========================================
Files 1866 1866
Lines 201416 201418 +2
==========================================
- Hits 162716 162715 -1
- Misses 38700 38703 +3 |
test/test_fx.py
Outdated
@@ -1139,6 +1139,23 @@ def forward(self): | |||
m = M() | |||
self.checkGraphModule(m, ()) | |||
|
|||
def test_torchbind_class_attribute_in_fx(self): | |||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can replace this try/catch with this incantation:
Line 300 in fadec77
if TEST_WITH_ROCM or IS_SANDCASTLE or IS_WINDOWS or IS_MACOS: |
@@ -2,6 +2,7 @@ | |||
from types import CodeType, FunctionType | |||
from typing import Any, Dict, Optional, List, Callable, Union | |||
import torch | |||
from torch._C import ScriptObject # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the lint error here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's Error: Module 'torch._C' has no attribute 'ScriptModule' [attr-defined]
Tried to add it in the mypy.ini for ignore import error, but it didn't work. A little bit hesitate to mark it ignore all the errors. So just mark this linne.
…48732) Summary: Pull Request resolved: pytorch#48732 add support for ScriptObject as attributes in symbolic trace. Test Plan: OSS CI Reviewed By: jamesr66a Differential Revision: D25116185 fbshipit-source-id: 24c18457934fb78c81800dee6f5dbf1349fcdaf1
This pull request was exported from Phabricator. Differential Revision: D25116185 |
5f551bd
to
7197f35
Compare
This pull request has been merged in 212ec07. |
Summary: add support for ScriptObject as attributes in symbolic trace.
Test Plan: OSS CI
Differential Revision: D25116185